Conversation
Added CapyrX thorns configuration to spacetimex.th
|
Ticket is here |
chcheng3
left a comment
There was a problem hiding this comment.
This PR looks good to me, I only have a few minor suggestions. I also appreciate that besides adding Multipatch support, the PR also makes the kernel in NewRadX_Apply easier to read by providing inline functions for computing spatial derivatives.
There was a problem hiding this comment.
Is there a use case where one would want to steer the parameter apply_inner_boundary?
There was a problem hiding this comment.
Personally, I can't foresee such use case at the moment, but it may be required by someone in the future.
There was a problem hiding this comment.
Can you add descriptions for the new arguments introduced for the Multipatch version of NewRadX_Apply? Namely, I would like to know the what the quantities vcoordx, vJ_da_dx, etc represent. Where would they be provided? Would a thorn implementing a Multipatch infrastructure provide them, e.g. CapyrX?
There was a problem hiding this comment.
I've added new docstrings, following the format of the original function. Please take a look and let me know if you'd like anything changed.
| * @param cctkGH Pointer to Cactus grid hierarchy struct. | ||
| * @param var State variable which will have boundary conditions applied to it. | ||
| * @param rhs RHS of the evolution equation for @param var | ||
| * @param vcoordx Global x vertex coordinates grid function. Providade by |
There was a problem hiding this comment.
I think "Providade" might be a typo on lines 39, 41, 43
This PR adds Multipatch support to NewRadX.
This support is done in a way that no specific multipatch driver is required, i.e., it does not require CapyrX to function. It also assumes no particular patch structure or that a special radial coordinate is available. The new functionality is provided via an overloaded function of the original, and all features are opt-in.
A new parameter has been introduced, which allows one to use the BC in inner boundaries, if the patch system contains them.